feat(secret): add --shared-with-me list mode for recipient discovery - #153
feat(secret): add --shared-with-me list mode for recipient discovery#153c1-squire-dev[bot] wants to merge 3 commits into
Conversation
Add cone secret list --shared-with-me, calling the caller-bound POST /api/v1/search/secrets/shared_with_me operation through the generated SDK (PaperSecret.SearchSecretsSharedWithMe). The default creator list (SearchMySecrets), its flags, help, and output stay unchanged; the new mode preserves pagination, query/status/type filters, enforces the endpoint's page_size<=100 and query<=256 limits, defaults include_own=false (opt-in via --include-own), and rejects an explicit --sharing-mode filter instead of silently dropping it, since the endpoint accepts no user_id, sort_by, or sharing_mode. Generalize the SDK-error-to-HTTPError mapping (mapPaperSecretCreateError -> mapPaperSecretError) so the shared search reports HTTP failures with the same shape as create. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
…redWithMe build Pin github.com/conductorone/conductorone-sdk-go to v1.29.1-0.20260905002051-ef0d92d9c5f2 (the speakeasy-sdk-regen branch head carrying the generated PaperSecret.SearchSecretsSharedWithMe operation and its request/response models) and re-vendor. This is the established SDK generation output from the refreshed canonical OpenAPI input (insulator now serves the C1 main canonical spec including the shared_with_me route); no generated file is hand-edited. Re-pin to the v1.29.1 tag once conductorone-sdk-go PR #117 merges and publishes. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| filippo.io/age v1.3.1 | ||
| github.com/conductorone/baton-sdk v0.3.17 | ||
| github.com/conductorone/conductorone-sdk-go v1.29.0 | ||
| github.com/conductorone/conductorone-sdk-go v1.29.1-0.20260905002051-ef0d92d9c5f2 |
There was a problem hiding this comment.
🟡 Suggestion: This pins conductorone-sdk-go to a pseudo-version on the unmerged speakeasy-sdk-regen-1784593459 branch rather than a published tag. Because that branch head is not reachable from any released ref, a force-push or branch deletion breaks go mod download/go mod verify and any non-vendored build (GOFLAGS=-mod=mod), and a release cut from this commit would ship an unreviewed pre-release SDK. Worth gating merge on the v1.29.1 tag landing so the re-pin is done here rather than as a follow-up. (confidence: high)
| // or sharing-mode filter, so those incompatibilities are rejected here rather | ||
| // than silently dropped. | ||
| func buildSearchSecretsSharedWithMeRequest(v *viper.Viper, cmd *cobra.Command) (*shared.PaperSecretServiceSearchSecretsSharedWithMeRequest, error) { | ||
| if cmd.Flags().Changed(secretSharingFlag) { |
There was a problem hiding this comment.
🟡 Suggestion: The incompatibility guards use cmd.Flags().Changed(...), but every value in this file is read through viper, which also resolves CONE_SHARING_MODE/CONE_INCLUDE_OWN env vars and profiles.<name>.sharing-mode config keys (see getSubViperForProfile in config.go). A user who sets sharing-mode via env or profile config gets it silently dropped in --shared-with-me mode instead of the clear error this is meant to produce, and include-own set the same way is silently ignored at line 646 while still being honored by v.GetBool(includeOwnFlag) at line 751. Consider gating on v.GetString(secretSharingFlag) != allFilter / v.GetBool(includeOwnFlag) so the guard matches how the values are actually read. (confidence: high)
| PageSize: &pageSize, | ||
| } | ||
| if query := strings.TrimSpace(v.GetString(queryFlag)); query != "" { | ||
| if len(query) > 256 { |
There was a problem hiding this comment.
🟡 Suggestion: len(query) counts bytes, not characters. If the endpoint's limit is 256 characters, a valid non-ASCII query (e.g. 100 CJK characters = 300 bytes) is rejected client-side before it ever reaches the API. Use utf8.RuneCountInString(query) > 256 if the contract is character-based. (confidence: medium — depends on whether the server counts bytes or runes)
| for { | ||
| resp, err := c.sdk.PaperSecret.SearchSecretsSharedWithMe(ctx, req) | ||
| if err != nil { | ||
| return nil, mapPaperSecretError(err) | ||
| } | ||
| if err := NewHTTPError(resp.RawResponse); err != nil { | ||
| return nil, err | ||
| } | ||
| if resp.PaperSecretServiceSearchResponse != nil { | ||
| out = append(out, resp.PaperSecretServiceSearchResponse.List...) | ||
| token := StringFromPtr(resp.PaperSecretServiceSearchResponse.NextPageToken) | ||
| if token != "" { | ||
| req.PageToken = &token | ||
| continue | ||
| } | ||
| } | ||
| return out, nil |
There was a problem hiding this comment.
🟡 Suggestion: Two things about this loop, both inherited from SearchMySecrets but now on a newly exported C1Client method. First, it mutates the caller's request in place (req.PageToken = &token), so a caller who reuses a request value gets a stale page token on the next call — copying req locally before looping would keep the method side-effect free. Second, the loop is unbounded: a server that keeps returning the same non-empty nextPageToken spins forever while out grows without limit. A page cap or a check that the token actually changed would bound it. (confidence: medium)
| // secretListRunForTest executes the same flag-routing core secretListRun uses, | ||
| // against the harness, without the authenticated cmdContext. | ||
| func secretListRunForTest(ctx context.Context, h *sharedListHarness, v *viper.Viper, cmd *cobra.Command) error { | ||
| if v.GetBool(sharedWithMeFlag) { | ||
| return secretListSharedWithMeRun(ctx, h, v, cmd) | ||
| } | ||
| req, err := buildSearchMySecretsRequest(v) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| _, err = h.SearchMySecrets(ctx, req) | ||
| return err |
There was a problem hiding this comment.
🟡 Suggestion: secretListRunForTest re-implements secretListRun's routing rather than exercising it, so the tests validate a copy that can drift from production. Concretely, the --include-own requires --shared-with-me guard (secret.go:646-648) has no coverage at all because this copy omits it. Extracting the post-cmdContext body of secretListRun into a helper that takes (ctx, creator, sharer, v, cmd) and calling that from both places would close the gap. (confidence: high)
General PR Review: feat(secret): add --shared-with-me list mode for recipient discoveryBlocking Issues: 0 | Suggestions: 3 | Threads Resolved: 0 Review SummaryThe new commit does two things: it switches the Security IssuesNone found. Correctness IssuesNone found. Suggestions
Prompt for AI agents |
…n routing Address review feedback on PR #153: - The shared-with-me query limit now counts Unicode code points (utf8.RuneCountInString) matching the server's protoc-gen-validate max_len:256 semantics, not UTF-8 bytes. A 256-character multibyte query (512 bytes) passes; 257 characters fails. Regression tests cover the multibyte boundary both sides. - Routing tests now drive the production runSecretList core (extracted from secretListRun so the branch selection, include-own-without- shared-with-me rejection, and creator-path contract are the exact code the CLI executes) instead of a duplicated dispatch. New tests pin the include-own guard and the creator path's page-size-1000 / created-desc sort / sharing-mode-allowed contract. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
1 participant
Summary
Adds
cone secret list --shared-with-me, the recipient-discovery path Phase 1 needs: a recipient discovers secrets shared with them and can then view one by its returned vault ID using the existingsecret view.POST /api/v1/search/secrets/shared_with_meoperation (PaperSecret.SearchSecretsSharedWithMe) through the generated SDK — continuing the architecture from Replace PaperSecret bridge with generated SDK #148; no handwritten bridge.secret listbehavior, flags, help, and output are unchanged (creator list viaSearchMySecrets).next_page_tokento exhaustion) and--query/--status/--typefilters across pages.--page-sizemax 100,--querymax 256 chars, nouser_id/sort_by/sharing_modeever sent; an explicit--sharing-modeis rejected with a clear error instead of silently dropped.include_owndefaults to false (the endpoint default), opt-in via--include-own(rejected without--shared-with-me).mapPaperSecretCreateError→mapPaperSecretError) so HTTP failures surface as coneHTTPErroruniformly.Dependency note
conductorone-sdk-gois pinned tov1.29.1-0.20260905002051-ef0d92d9c5f2— thespeakeasy-sdk-regen-1784593459branch head (conductorone-sdk-go PR #117) generated by the established nightly Speakeasy workflow from the refreshed canonical OpenAPI input, which now includes theshared_with_meroute. Once #117 merges and thev1.29.1tag publishes, re-pin to the tag (mechanicalgo get+go mod vendor).Tests
--shared-with-meselects the shared endpoint and never sendsuserId/sortBy/sharingMode(asserted on the wire via httptest)pageTokenthreaded--sharing-moderejected;--include-owndefault/opt-ingo test ./...,go vet ./..., golangci-lint (0 issues) all pass locally